Skip to content

fix: move LLMError into the llm package (#1096)#1112

Open
CoronRing wants to merge 3 commits into
mainfrom
fix/1096-llm-error-package-separation
Open

fix: move LLMError into the llm package (#1096)#1112
CoronRing wants to merge 3 commits into
mainfrom
fix/1096-llm-error-package-separation

Conversation

@CoronRing
Copy link
Copy Markdown
Contributor

@CoronRing CoronRing commented May 21, 2026

Summary

  • Moves LLMError from railtracks.exceptions into railtracks.llm.errors, inheriting from RTLLMError instead of RTError, so the llm package no longer imports from railtracks.exceptions
  • Also removes a secondary violation: NodeInvocationError (a railtracks exception) was being raised inside llm/models/_litellm_wrapper.py; replaced with ModelError from within the llm package
  • Updates all consumers (built_nodes, tests) to import LLMError from railtracks.llm or railtracks.llm.errors
  • Exports LLMError from railtracks.llm.__init__ for discoverability

Files changed

File Change
llm/errors.py New — LLMError(RTLLMError) definition
llm/__init__.py Export LLMError
llm/models/_litellm_wrapper.py Import LLMError from ..errors; replace NodeInvocationError with ModelError
exceptions/errors.py Remove LLMError class
exceptions/__init__.py Remove LLMError export
built_nodes/concrete/*.py Update imports
3× test files Update imports; update NodeInvocationErrorModelError test expectation

Test plan

  • test_litellm_wrapper.py — all tests pass, including the updated ModelError expectation for invalid tool parameters
  • test_structured_tool_call_llm.py — all tests pass
  • test_exceptions.pyLLMError tests pass with new location
  • Full unit suite: 1508 passed

Closes #1096

@CoronRing CoronRing force-pushed the fix/1096-llm-error-package-separation branch 2 times, most recently from 8b7d2e6 to a01c83a Compare May 21, 2026 20:01
LLMError now lives in railtracks.llm.errors and inherits from RTLLMError,
eliminating the dependency the llm package had on railtracks.exceptions.
Also replaces a NodeInvocationError raised inside _litellm_wrapper with
ModelError to fix the same cross-package violation. All built_nodes,
related tests, and exception tests updated to import from the new location.

Closes #1096
@CoronRing CoronRing force-pushed the fix/1096-llm-error-package-separation branch from a01c83a to 3174685 Compare May 21, 2026 20:05
@CoronRing
Copy link
Copy Markdown
Contributor Author

This PR may be addressed by the middleware logics, hence may not need to be addressed right now. @soulFood5632 let me know your thoughts.

@CoronRing CoronRing requested a review from Copilot May 27, 2026 00:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

LLMError was moved out of railtracks.exceptions into railtracks.llm.errors
as part of the llm package separation (#1096). Update docs/scripts/error_handling.py
to import from the new location.
@CoronRing CoronRing marked this pull request as ready for review May 27, 2026 00:38
Copy link
Copy Markdown
Member

@soulFood5632 soulFood5632 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor questions

self.message_hist.append(UserMessage(content=user_message))
else:
# the message is malformed from the model
raise LLMError(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THis wasn't super clear from the ticket description so I apologize. Any errors raised in railtracks core should be railtracks errors. It's fine if we pass the LLM error through, but any new error we raise should be a railtracks error.

notes=[
"Tool.parameters must be a set of Parameter objects",
],
raise ModelError(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this error changed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Tech Debt] Cross Dependency in Errors module

3 participants